fix: unsynchronized concurrent wasm calls #1155
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -73,7 +85,7 @@ function Home() { | |||
| return <></>; | |||
| } | |||
|
|
|||
| if (controller.session(policies)) { | |||
| if (hasSessionForPolicies) { | |||
There was a problem hiding this comment.
Hmm does this migration to async maintain the same behavior? It seems on initial render it will be false and proceed now. I think we might need a loading state for the intermediate state, when the useEffect hasn't been executed
There was a problem hiding this comment.
I think you're right that this is technically not the exact same behaviour as the sync version. However, I think under a real world scenario it's very unlikely that users would ever run into the case where the loading state is observable. This is because when .connect() is called, it's highly unlikely that the host app would somehow also be blocking the wasm module. After all, .connect() is likely their first function call before doing anything real. This means that despite being async, the hasSessionForPolicies state would instantaneously be set to its desired value.
That said, I can certainly add a loading state. It's just that given the unlikeliness of it being actually observable as discussed above, I'd go for something very primitive. I will test it by artificially delaying the wasm call.
There was a problem hiding this comment.
Alright I just pushed a commit adding a loading screen using the existing <PageLoading /> component:
|
Got conflicts. Rebasing now. |
xJonathanLEI
force-pushed
the
fix/wasm_internal_mutability
branch
from
cca72f8 to
6e47dcb
Compare
|
Rebased. |
|
this is also happening on dojo.js torii client |
Concurrent calls to the wasm module where at least one call borrows
CartridgeAccountmutably causeswasm-bindgen's runtime borrow checker to throw:This PR fixes it by:
CartridgeAccountWithMetatype is now used for reading fixed values. This type is only ever borrowed immutably, so concurrent accesses are fine.controllervalue with a Promise-backedWasmMutex, and changing all entrypoint signatures to use&selfinstead of&mut self.Note
Using a mutex for all calls, as implemented in this PR, is an overkill. A more ideal implementation would be something like a
RwLocksuch that concurrent reads are still allowed.